Repository navigation
Feat/peg deviation alerts - #322
p70436464-prog wants to merge 4 commits into
Conversation
…s venues
- Calculate proportions per venue for a given size that minimise total slippage
- Return split proportions only when splitting beats the single-venue route
- Include single-venue alternative comparison in response
- Add GET /price/:assetA/:assetB/split route filtering on and echoing network
- Fully declare /price/{assetA}/{assetB}/split and schemas in openapi.yaml and openapi.json
- Add vitest tests covering size where splitting wins and where it does not
|
@p70436464-prog Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
|
CI is red, and both of your open PRs fail on the same file for the same reason —
const { mockThing } = vi.hoisted(() => ({ mockThing: vi.fn() }))
vi.mock('../network/clients', () => ({ getHorizonServer: mockThing }))
There is a second trap behind this one, specific to mocking Separately: #320 and #322 both carry |
tsc --noEmit clean vitest 97 files, 1135 passing dropped-tests none #311 adaptive slippage impact endpoint #323 trade size and volume concentration metrics #320 and #322 screened clean individually but conflict once #311 and #323 are in -- all four touch the same routing and pricing surface. They need a rebase rather than a fix; noted on both. Claude-Session: https://claude.ai/code/session_01USgemLt4Rnz4SGB1Srf3GB
Miracle656
left a comment
Miracle656
left a comment
There was a problem hiding this comment.
Needs a rebase. This screened clean on its own — tsc --noEmit passes against main as it was — but conflicts once #311 (adaptive slippage) and #323 (trade size and volume concentration) are in, and those landed as b906b47.
All four touch the same routing and pricing surface, which is why they collided rather than merged. Nothing is wrong with the code.
Rebase on current main and re-run npx vitest run before re-requesting. Two things worth checking while you are there, because both have caught PRs in this repo today:
- the logging-hygiene test forbids
console.*in non-test source — usecreateLogger(three PRs tripped this in two batches) tests/openapi.test.tsfails if you document a route without registering it insrc/app.ts, or the reverse
Happy to take it as soon as it is green.
Miracle656
left a comment
Miracle656
left a comment
There was a problem hiding this comment.
Scanned clean, and it rebases onto current main with no conflicts. Two things to sort out, one mechanical and one about how this PR is shaped.
1. It does not compile — same five errors as #320, for the same reason
This branch carries both features: src/alerts/pegDeviation.ts (432 lines, the peg work) and src/aggregator/splitRoute.ts (351 lines, which is #320). So it inherits #320's breakage:
src/aggregator/splitRoute.ts(6-9): '"../types"' has no exported member
'SplitRecommendation' / 'SplitVenueAllocation' / 'SplitComparison' / 'SplitProvenance'
src/api/rest.ts(20): '"./schemas"' has no exported member 'splitResponseSchema'
Neither this branch nor main defines those five symbols — the branch does not touch src/types.ts or src/api/schemas.ts at all. They were almost certainly dropped in a rebase that took main's side of src/types.ts wholesale, which leaves no conflict marker behind to notice. Check your reflog before rewriting them; they likely still exist in a pre-rebase commit.
The good news: your peg code is clean. All five errors are in the split-routing files. tsc has no complaint about src/alerts/pegDeviation.ts.
2. Decide which PR carries what
Right now #320 and #322 are the same branch content, so they will always break and fix together, and merging either merges both. That is worth untangling, because the peg work is ready and the split work is not:
- If you split them, the peg alerting could land on its own as soon as this compiles, and
/splitcan take as long as it needs. - If you would rather keep one branch, close one of the two PRs so there is a single place to review, and say which.
Your call either way — I am not closing either one.
On the peg code itself
The state machine (pegged / deviating / depegged / recovering) with explicit deviation and recovery event types is the right model. An alert that fires on departure but never tells you it came back is the kind operators mute within a week, so having recovery as a first-class event rather than an absence is the detail that makes this usable.
Worth knowing: #353 is open and adds GET /pegs (src/routes/pegs.ts), comparative peg deviation monitoring. I read both — they are complementary rather than duplicates, yours being the alerting engine and theirs the read endpoint. But you are both modelling "how far from peg is this", so it is worth one of you looking at the other's thresholds before both land, so the route and the alerts cannot disagree about what counts as depegged.
Get npx tsc --noEmit clean and re-request.
Closes #288
Summary
Implements a peg-deviation alert in
src/alerts/through the existing alerts path to monitor stablecoins and pegged assets leaving and returning to their peg.What was built
PegDeviationTracker,checkPegDeviation): Evaluates price ticks against a configurable target price and deviation band (band,bandBps,bandPct).sustainedDurationMsacross multiple samples before raising an alert, ensuring a single bad tick does not fire.recoveryevent when price recovers within the peg band, detailingrecoveredAt,deviationStartedAt, and totaldurationMs.venues: string[]) and full provenance ({ venues, sampleCount, freshnessMs }).deliverJsonWithRetriesfor HMAC-signed webhook delivery.Testing
tests/pegDeviation.test.ts: Added test suite proving that: